GH-36845: [C++][Python] Allow type promotion on pa.concat_tables
#36846
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
github-actions
bot
added
Component: C++
Component: Python
awaiting review
Fokko
changed the title
pa.concat_tables
github-actions
bot
added
awaiting change review
python/pyarrow/tests/test_table.py
Outdated
| t2 = pa.Table.from_arrays( | ||
| [pa.array([1.0, 2.0], type=pa.float32())], ["float_field"]) | ||
|
|
||
| result = pa.concat_tables([t1, t2], promote=True) |
There was a problem hiding this comment.
Suggested change
| result = pa.concat_tables([t1, t2], promote=True) | |
| with pytest.warns(FutureWarning): | |
| result = pa.concat_tables([t1, t2], promote=True) |
This asserts the warning is raised and at the same time also ensures we don't unnecessarily see the warning in the pytest logs
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
jorisvandenbossche
approved these changes
Oct 10, 2023
|
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 5f57219. There were 2 benchmark results indicating a performance regression:
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them. |
|
FYI, those reported performance regressions were just flakes. The timings are still stable at the same level for later commits. |
JerAguilon
pushed a commit
to JerAguilon/arrow
that referenced
this pull request
Oct 23, 2023
…es` (apache#36846) Revival of apache#12000 ### Rationale for this change It would be great to be able to do promotions when `concat`'ing a table, such as: ```python def test_concat_tables_with_promotion_int(): import pyarrow as pa t1 = pa.Table.from_arrays( [pa.array([1, 2], type=pa.int64())], ["int"]) t2 = pa.Table.from_arrays( [pa.array([3, 4], type=pa.int32())], ["int"]) result = pa.concat_tables([t1, t2], promote=True) assert result.equals(pa.Table.from_arrays([ pa.array([1, 2, 3, 4], type=pa.int64()) ], ["int"])) ``` ### What changes are included in this PR? ### Are these changes tested? ### Are there any user-facing changes? * Closes: apache#36845 Lead-authored-by: Fokko Driesprong <[email protected]> Co-authored-by: David Li <[email protected]> Co-authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Joris Van den Bossche <[email protected]>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
…es` (apache#36846) Revival of apache#12000 ### Rationale for this change It would be great to be able to do promotions when `concat`'ing a table, such as: ```python def test_concat_tables_with_promotion_int(): import pyarrow as pa t1 = pa.Table.from_arrays( [pa.array([1, 2], type=pa.int64())], ["int"]) t2 = pa.Table.from_arrays( [pa.array([3, 4], type=pa.int32())], ["int"]) result = pa.concat_tables([t1, t2], promote=True) assert result.equals(pa.Table.from_arrays([ pa.array([1, 2, 3, 4], type=pa.int64()) ], ["int"])) ``` ### What changes are included in this PR? ### Are these changes tested? ### Are there any user-facing changes? * Closes: apache#36845 Lead-authored-by: Fokko Driesprong <[email protected]> Co-authored-by: David Li <[email protected]> Co-authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Joris Van den Bossche <[email protected]>
dgreiss
pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
…es` (apache#36846) Revival of apache#12000 ### Rationale for this change It would be great to be able to do promotions when `concat`'ing a table, such as: ```python def test_concat_tables_with_promotion_int(): import pyarrow as pa t1 = pa.Table.from_arrays( [pa.array([1, 2], type=pa.int64())], ["int"]) t2 = pa.Table.from_arrays( [pa.array([3, 4], type=pa.int32())], ["int"]) result = pa.concat_tables([t1, t2], promote=True) assert result.equals(pa.Table.from_arrays([ pa.array([1, 2, 3, 4], type=pa.int64()) ], ["int"])) ``` ### What changes are included in this PR? ### Are these changes tested? ### Are there any user-facing changes? * Closes: apache#36845 Lead-authored-by: Fokko Driesprong <[email protected]> Co-authored-by: David Li <[email protected]> Co-authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Joris Van den Bossche <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Revival of #12000
Rationale for this change
It would be great to be able to do promotions when
concat'ing a table, such as:What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?